Drain per-host reservation when a VM starts on a different host by Kukunin · Pull Request #13363 · apache/cloudstack

Kukunin · 2026-06-06T08:27:08Z

What's happening

When a VM is stopped via the API, CloudStack moves its CPU/RAM from used to reserved on its last_host_id (see the Stopping → Stopped + OperationSucceeded branch in CapacityManagerImpl.postStateTransitionEvent). The idea is that a quick restart on the same host can reclaim its earmarked slot cheaply.

The asymmetry: when the VM later starts on the same host, the fromLastHost=true branch in allocateVmCapacity drains that reservation. When it starts on a different host, the old reservation just sits there. It only clears when:

capacity.skipcounting.hours (default 1h) elapses and updateCapacityForHost recycles it, or
the VM is destroyed/expunged.

Until then, the orphan reservation gets summed into the cluster's used + reserved aggregate by FirstFitPlanner.removeClustersCrossingThreshold. On a cluster that's already near cluster.memory.allocated.capacity.disablethreshold (default 0.85), the phantom can trip the threshold and block subsequent VM starts in the whole cluster — even though the VM in question isn't actually consuming anything on its old host.

We hit this on a fairly full cluster where stop-then-start cycles started failing intermittently with InsufficientServerCapacityException: No destination found. The "ghost" capacity was the released-but-not-drained reservation from the last stop.

The fix

postStateTransitionEvent now drains the VM's reservation on its previous host before allocating on the target host — regardless of whether the target is the same host or a different one. Treating both cases identically removes the fromLastHost asymmetry.

if ((newState == State.Starting || newState == State.Migrating || event == Event.AgentReportMigrated) && vm.getHostId() != null) {
    if (vm.getLastHostId() != null) {
        releaseVmCapacity(vm, true, false, vm.getLastHostId());
    }
    allocateVmCapacity(vm);
}

Side effects of unifying the path:

The fromLastHost=true branch in allocateVmCapacity is now unreachable from postStateTransitionEvent. The only other caller (VirtualMachineManagerImpl#reconfiguringOnExistingHost) already passes false, so the parameter is removed entirely.
Fixes the long-standing moveToReservered typo (3 e's) — renamed to moveToReserved throughout the interface, the impl, and the debug logs.
One logger.debug(String.format(...)) in postStateTransitionEvent switched to SLF4J {} placeholders to match the rest of the file.

Tests

Two new tests in CapacityManagerImplTest cover both transitions:

testPostStateTransitionReleasesStaleReservationWhenStartingOnDifferentHost — fails on main, passes with the fix. Verifies releaseVmCapacity(vm, true, false, oldHostId) is invoked.
testPostStateTransitionReleasesReservationWhenStartingOnSameHost — guards the unified contract so both paths stay in lockstep.

Manual validation

Reproduced on a small test cluster:

Deploy a VM on host A, stop it (reserved=128M appears on A in op_host_capacity).
Force a start on host B via hostid.
Logs show the inline drain: release mem from host: A, old reserved: 128MB → new reserved: 0.
After the VM reaches Running on B, last_host_id updates to B and updateCapacityForHost agrees the reservation is gone.

Same-host stop/start round-trip continues to behave the same way as before.

When a VM is stopped via the API, postStateTransitionEvent moves its used capacity into reserved_capacity on its last host (so a quick restart can reclaim it cheaply). Until now, this reservation was only drained when the VM started again on the same lastHostId. Starting on any other host left an orphan reservation that lingered for up to one hour (capacity.skipcounting.hours) and was summed into the cluster used+reserved aggregate by FirstFitPlanner.removeClustersCrossingThreshold, spuriously tripping the disable-threshold and blocking later starts. Always drain the VM's reservation on its previous host before allocating on the target host — same host or not. This removes the fromLastHost branch (now dead, the only other caller already passes false), the matching boolean parameter on allocateVmCapacity, and the misspelt moveToReservered parameter everywhere it appears. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

DaanHoogland · 2026-06-15T10:48:36Z

@blueorangutan package

blueorangutan · 2026-06-15T10:50:05Z

@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

Copilot

Pull request overview

This PR fixes a capacity-accounting asymmetry where per-host reserved CPU/RAM left behind on a VM’s previous host could linger after the VM starts on a different host, inflating used+reserved and potentially blocking subsequent placements at the cluster threshold.

Changes:

Drain the VM’s reserved capacity on its last_host_id during relevant state transitions before allocating capacity on the target host.
Simplify capacity allocation by removing the now-unreachable fromLastHost path and updating callers accordingly.
Add unit tests to ensure reservations are released for both same-host and different-host starts; also standardize logging and fix a long-standing moveToReservered typo.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
server/src/main/java/com/cloud/capacity/CapacityManagerImpl.java	Unifies reservation draining behavior on start/migrate transitions; removes `fromLastHost` logic; adjusts logging/typo.
engine/components-api/src/main/java/com/cloud/capacity/CapacityManager.java	Updates the public API to remove the `allocateVmCapacity(..., fromLastHost)` parameter and fixes the `moveToReservered` typo.
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java	Updates call site to the new `allocateVmCapacity(vm)` signature.
server/src/test/java/com/cloud/capacity/CapacityManagerImplTest.java	Adds regression tests covering reservation draining on same-host and different-host starts.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

        HostVO host = _hostDao.findById(hostId);
        if (HypervisorType.External.equals(host.getHypervisorType())) {
            return true;
        }
-        return releaseVmCapacity(vm, moveFromReserved, moveToReservered, host);
+        return releaseVmCapacity(vm, moveFromReserved, moveToReserved, host);


      Host lastHost = _hostDao.findById(vm.getLastHostId());
      Host oldHost = _hostDao.findById(oldHostId);
      Host newHost = _hostDao.findById(vm.getHostId());


      if ((newState == State.Starting || newState == State.Migrating || event == Event.AgentReportMigrated) && vm.getHostId() != null) {
-        boolean fromLastHost = false;
-        if (vm.getHostId().equals(vm.getLastHostId())) {
-          logger.debug("VM starting again on the last host it was stopped on");
-          fromLastHost = true;
+        if (vm.getLastHostId() != null) {
+          releaseVmCapacity(vm, true, false, vm.getLastHostId());
        }
-        allocateVmCapacity(vm, fromLastHost);
+        allocateVmCapacity(vm);
      }


blueorangutan · 2026-06-15T11:42:13Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18263

Two defensive fixes from Copilot's review of PR apache#13363: 1. releaseVmCapacity(VM, ..., Long hostId): _hostDao.findById(hostId) can return null when last_host_id points to a deleted host. The subsequent host.getHypervisorType() check would NPE. The Host overload already null-checks AND has the External check, so delegate to it instead of duplicating the External check here. 2. postStateTransitionEvent: the lastHost was being fetched twice (once for logging at L942, again inside releaseVmCapacity via the Long overload at L987). Reuse the already-resolved Host. The Host overload also handles null, so the explicit null check on getLastHostId() becomes redundant. (Skipped Copilot's third suggestion to guard findById(null) — the GenericDaoBase.findById(ID, boolean, Boolean) implementation at GenericDaoBase.java:1066 explicitly returns null for null id, so the existing code is already safe.) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

boring-cyborg Bot added component:compute component:orchestration labels Jun 6, 2026

This was referenced Jun 6, 2026

[repo-status] Daily Status Report – June 6, 2026 #13364

Closed

[repo-status] Daily Status Report – June 7, 2026 #13367

Closed

DaanHoogland requested review from JoaoJandre, Copilot and shwstppr and removed request for shwstppr June 15, 2026 10:46

Copilot started reviewing on behalf of DaanHoogland June 15, 2026 10:47 View session

Copilot AI reviewed Jun 15, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Drain per-host reservation when a VM starts on a different host#13363

Drain per-host reservation when a VM starts on a different host#13363
Kukunin wants to merge 2 commits into
apache:mainfrom
Kukunin:fix-stale-host-reservation-on-cross-host-start

Kukunin commented Jun 6, 2026

Uh oh!

DaanHoogland commented Jun 15, 2026

Uh oh!

blueorangutan commented Jun 15, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

blueorangutan commented Jun 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

Kukunin commented Jun 6, 2026

What's happening

The fix

Tests

Manual validation

Uh oh!

DaanHoogland commented Jun 15, 2026

Uh oh!

blueorangutan commented Jun 15, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

blueorangutan commented Jun 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants